Skip to content

Minor optimization to hot minimap procs - #12747

Open
fira wants to merge 3 commits into
cmss13-devs:masterfrom
fira:minimap_on_move_optimization
Open

Minor optimization to hot minimap procs#12747
fira wants to merge 3 commits into
cmss13-devs:masterfrom
fira:minimap_on_move_optimization

Conversation

@fira

@fira fira commented Jul 27, 2026

Copy link
Copy Markdown
Member

About the pull request

Minor optimization to /image/proc/minimap_on_move

The proc is both hot and extremely costly for some reason.
During recent 200-pop round it hit about 82,000 ms over 510,000 calls.

The only real highlight is it reuses the result of minimap lookup instead of doing it twice, because I don't see what else could be expensive here unless it's literally setting pixel_x/y.

--

Same treatment for /datum/controller/subsystem/minimaps/proc/remove_marker which is vastly less expensive, but still expensive

🆑
code: Improved minimap_on_move and remove_marker performance
/:cl:

@github-project-automation github-project-automation Bot moved this to Awaiting Review in Review Backlog Jul 27, 2026
@fira fira changed the title perf optimization Optimizes /image/proc/minimap_on_move Jul 27, 2026
@cmss13-ci cmss13-ci Bot added Missing Changelog Maintainers always document their changes. size/XS Denotes a PR that changes 0-19 lines, ignoring generated files. labels Jul 27, 2026
@cmss13-ci cmss13-ci Bot added size/S Denotes a PR that changes 19-49 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-19 lines, ignoring generated files. labels Jul 27, 2026
@fira fira changed the title Optimizes /image/proc/minimap_on_move Minor optimization to hot minimap procs Jul 27, 2026
if(isturf(source.loc))
pixel_x = MINIMAP_PIXEL_FROM_WORLD(source.x) + SSminimaps.minimaps_by_z["[source.z]"].x_offset
pixel_y = MINIMAP_PIXEL_FROM_WORLD(source.y) + SSminimaps.minimaps_by_z["[source.z]"].y_offset
if(source.z)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a neat way to check if an atom is on a turf, might be worth exporting this check to a helper define

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i use that everywhere, figure it's simpler and faster.
Technically it can fail in one way, if you nullspace it, you can VV z only to a value and it'll still be nullspaced since x/y are 0. But that's obviously not supposed to happen, just like you aren't supposed to change loc.

@github-project-automation github-project-automation Bot moved this from Awaiting Review to Partially Approved / Mergeable in Review Backlog Jul 27, 2026
@Drulikar Drulikar added the Performance the game sucks and this makes it suck less label Jul 28, 2026
@cmss13-ci cmss13-ci Bot added Code Improvement Make the code longer and removed Missing Changelog Maintainers always document their changes. labels Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Improvement Make the code longer Performance the game sucks and this makes it suck less size/S Denotes a PR that changes 19-49 lines, ignoring generated files.

Projects

Status: Partially Approved / Mergeable

Development

Successfully merging this pull request may close these issues.

3 participants